Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Soundness fixes for RUSTSEC-2020-0041 #15

Closed
wants to merge 5 commits into from
Closed

Conversation

kornelski
Copy link
Collaborator

Fixes #11

@Qwaz @vorner do you want to have a look at it?

@vorner
Copy link
Contributor

vorner commented Feb 12, 2021

I'll go over it later today.

Any reason why the fix I've sent #13 isn't good?

@vorner
Copy link
Contributor

vorner commented Feb 12, 2021

After going through it quickly (not proving anything):

  • I think one of the branches in the insert_from shifts existing elements to the right, creates a „hole“ in the middle and fills it with the items from the iterator. If the iterator is too short, even being careful to update the length after the fact instead of before won't solve it, since the actual elements are not consecutive in the array.
  • It's not strictly for that CVE/Rustsec, but at least when I was writing Fix soundness issues in sized chunks and ringbuffer #13, I must have thought the ringbuffer has a very similar set of problems as the sized chunk. I'm not claiming to be sure to be correct in that feeling, but do you want to check that one too?

Copy link

@Qwaz Qwaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe insert_from() still have safety problems to be fixed.

src/sized_chunk/mod.rs Show resolved Hide resolved
src/sized_chunk/mod.rs Show resolved Hide resolved
@kornelski kornelski closed this Feb 13, 2021
@kornelski kornelski deleted the rustsec-2020-0041 branch February 13, 2021 23:45
@kornelski
Copy link
Collaborator Author

I haven't noticed #13 before! That PR looks great, so I've merged that one instead. Thank you for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple soundness issues in Chunk and InlineArray
3 participants